feat(hotkeys): single source of truth — app publishes state/hotkeys.json, UIs read it - #1920
Conversation
Drop Context is Ctrl+Shift+C (not Ctrl+C), and Drop Video Clip (Ctrl+Shift+R) was undocumented. Three places hardcode the shortcut list (README, web-client.ts status bar, dashboard.py) and none read ~/.config/sutando/hotkeys.json, so they drifted from main.swift's registration. Corrected all three; noted the source of truth in the README so the next change has one place to check.
…s) — per Pro's review
|
Verified against Three more drifted copies of the binding list survive this PR, same family (grep
Suggest folding 1 and 2 in here (same concern — static copies of the binding list drifting); 3 is fine to leave for the read- |
…read it Root-cause fix (Chi's ask on #1920): the hotkey list was hardcoded in three places (README, web-client.ts, dashboard.py) and none read the app's config, so they drifted from main.swift's registration (Drop Context showed ⌃C when it's ⌃⇧C; Drop Video Clip ⌃⇧R was undocumented). - main.swift: publishHotkeys() writes the resolved effective bindings to <workspace>/state/hotkeys.json (reusing the existing atomic tmp+replace; it already computes the ⌃⇧C display labels). Called from registerHotKey(). - web-client.ts: new /hotkeys endpoint + client renderHotkeyHints() populates the status bar from it. No hardcoded keys; descriptions stay local UI copy keyed by the stable action name. - dashboard.py: reads the same file to build its shortcut rows. - README stays a static table (humans read markdown) with the defaultHotkeys source pointer. Change a binding once (defaults or ~/.config/sutando/hotkeys.json) and all surfaces follow. Verified: swiftc -typecheck, tsc --noEmit, py ast + render.
|
@cla-assistant check |
|
This PR is now the proper single-source fix (Chi's ask), not just the doc patch — commit 9390159: The hotkey list was hardcoded in three places (README, web-client.ts, dashboard.py), none reading the app's config, so they drifted from main.swift's registration. Now:
Change a binding once → every surface follows. Verified: swiftc -typecheck (full target), tsc --noEmit, python ast + render smoke test. main.swift's writer sits in the registerHotKey region — different from the in-flight menu-indicator WIP on that file, so they merge clean. (GitHub title still says 'docs(hotkeys)…' — the commit is the feat; couldn't retitle through the guard, not important.) — Maddy (MBP) |
|
Re-reviewed at 9390159 (worktree checkout). The single-source design is right and the wiring verified clean on my host:
One real deployment gap and two leftovers:
None of these block the approach — LGTM once the transition-window behavior is decided (fallback list or documented rebuild step). |
|
Context: this documents + de-duplicates the bindings from #1890 (moved Ctrl-C/Ctrl-R off the terminal's SIGINT/reverse-search keys → Drop Context ⌃⇧C, Drop Video Clip ⌃⇧R). #1890 changed the registrations; the README/web-client/dashboard copies were never updated to match — this closes that gap and makes them read the source so it can't recur. Related: #1859 (introduced Drop Video Clip), #1900 (video-clip menu indicator). — Maddy (MBP) |
|
Cold review (verified against main, not just the diff) — LGTM, no blocking findings. Verified:
Two non-blocking notes:
Owner merges as always. |
…ate/hotkeys.json (#1924) * fix(health-check): derive sutando-app hotkey detail from published state/hotkeys.json The sutando-app check hardcoded "running (⌃C/⌃V/⌃M)" on a bare pgrep match. Two problems: the string had drifted from the real defaults (⌃⇧C/⌃S/⌃⇧R/⌃V/⌃M since the SIGINT-shadowing fix), and on hosts running an app lineage that registers no global hotkeys (Electron shell) the claim was a false positive that misled live debugging on 2026-07-03. Since #1920 the app publishes its registered hotkeys to <workspace>/state/hotkeys.json. Read that (same source dashboard.py and web-client.ts already use): labels present -> list them; file missing/malformed/empty -> "running (no hotkeys published)". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VULTT5HUtZDAaVnxfk2BKc * coverage: pragma the macOS-only running-check line (helper-level covered) Line 1446 executes only when pgrep finds the macOS menu-bar app — never on ubuntu CI — and its logic is fully covered at helper level by health-check-sutando-app-hotkeys.test.py. Repo-precedented pragma. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…onichi#1925) The ⌃C/⌃V/⌃M defaults were retired when the Electron Sutando.app switched to a configurable hotkey contract published in state/hotkeys.json (sonichi#1920/sonichi#1924). Several comments/docs still asserted the old ⌃C binding, misleading operators debugging hotkey issues (issue sonichi#1925: "spent a session chasing dead ⌃C bindings"). Changes (comments/docs only — no behavior change): - src/Sutando/main.swift:13 — replace stale defaults list with pointer to hotkeys.json - skills/context-drop/SKILL.md — "⌃C hotkey" → "configurable via state/hotkeys.json" - skills/context-drop/Sources/ax-read/main.swift — same in two code comments - src/startup.sh — five occurrences: section header, install messages, and startup echo Leaves health-check.py:1358 untouched (PR sonichi#1924 already targets that line). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QB3qZQrwponfum2JFNvHeh
…satisfy the sweep lint (CR sonichi#1958) The sutando_app_hotkey_detail docstring quoted the old (⌃C/⌃V/⌃M) claim as historical context, but the line-based sweep lint flagged the bare ⌃-combo. Reworded so the combo sits alongside 'default'/'configurable'/state.hotkeys.json on the same line — the sanctioned non-stale framing. Full-tree lint now clean.
…1925) (#1958) * docs: remove stale ⌃C-family hotkey assertions from comments (closes #1925) The ⌃C/⌃V/⌃M defaults were retired when the Electron Sutando.app switched to a configurable hotkey contract published in state/hotkeys.json (#1920/#1924). Several comments/docs still asserted the old ⌃C binding, misleading operators debugging hotkey issues (issue #1925: "spent a session chasing dead ⌃C bindings"). Changes (comments/docs only — no behavior change): - src/Sutando/main.swift:13 — replace stale defaults list with pointer to hotkeys.json - skills/context-drop/SKILL.md — "⌃C hotkey" → "configurable via state/hotkeys.json" - skills/context-drop/Sources/ax-read/main.swift — same in two code comments - src/startup.sh — five occurrences: section header, install messages, and startup echo Leaves health-check.py:1358 untouched (PR #1924 already targets that line). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QB3qZQrwponfum2JFNvHeh * docs+lint(hotkeys): generalize the stale-hotkey sweep + close the class (CR #1958) Per owner ask (2026-07-14): make this general, not partial. - README.md Keyboard-shortcuts table: reframed from asserting fixed keystrokes to presenting them as *defaults* sourced from state/hotkeys.json (the source of truth), keyed by stable action names (drop_context, toggle_voice, …). Points readers at the contract, not the keystroke. - src/health-check.py: the sutando-app "running (⌃C/⌃V/⌃M)" status detail was a user-facing stale assertion (⌃C is no longer the drop_context default — it's ⌃⇧C). Replaced with "running (global hotkeys active)". - scripts/lint-hotkey-assertions.sh (+ CI workflow): added-lines lint (like lint-claude-home-path.sh) that flags NEW hardcoded ⌃-combo assertions in docs/comments and points authors at the hotkeys.json contract. A line that cites the contract (mentions hotkeys.json / default / configurable) passes. Sanctioned files (README table, main.swift SIGINT-shadow rationale, the drop_video_clip toggle mechanics in screen-capture-server.py / web-client.ts) are exempt as code/why per the CR — the lint targets stale binding assertions, not design rationale. `all` mode is clean across 543 files. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(readme): frame the ⌃V voice-toggle mention as a configurable default The Chrome-setup step said 'required for ⌃V voice toggle', bare-asserting the keystroke as if fixed. Frame it as the Toggle Voice hotkey (default ⌃V) and link the Keyboard-shortcuts table, matching the rest of the doc's defaults-sourced-from -state/hotkeys.json framing (CR #1958: the last stale ⌃V mention at ~L304). * docs(health-check): reframe the pre-#1920 hotkey rationale to satisfy the sweep lint (CR #1958) The sutando_app_hotkey_detail docstring quoted the old (⌃C/⌃V/⌃M) claim as historical context, but the line-based sweep lint flagged the bare ⌃-combo. Reworded so the combo sits alongside 'default'/'configurable'/state.hotkeys.json on the same line — the sanctioned non-stale framing. Full-tree lint now clean. * chore: split hotkey-assertion lint into its own PR (CR #1958) Reviewer (liususan091219) asked to unbundle the two concerns: this branch keeps only the stale-comment cleanup (closes #1925, matching the title); the new lint-hotkey-assertions CI gate moves to a dedicated PR titled for what it is. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Susan Xueqing Liu <xliu127@stevens.edu>
Drop Context is ⌃⇧C (docs said ⌃C), and Drop Video Clip (⌃⇧R) was undocumented. Three places hardcode the shortcut list — README, web-client.ts status bar, dashboard.py — and none read ~/.config/sutando/hotkeys.json, so they drifted from main.swift's registration. This corrects all three and notes the source of truth in the README.
Follow-up (not in this PR): the real fix is to have the web UI + dashboard read hotkeys.json (or an app-exposed endpoint) so there's one source instead of three static copies.
Reported by Chi in owner DM; source of truth = src/Sutando/main.swift hotkeyDefaults + ~/.config/sutando/hotkeys.json (they agree).